Enhance logging tests and refactor log handling#333
Merged
Conversation
…s_logs.py to ensure _JsonExceptionFileHandler leaves pre-existing JSON content untouched while appending new exception entries.
…ies_logs.py to verify _JsonExceptionFileHandler.emit short-circuits cleanly when json.dumps raises TypeError, leaving the existing JSON file untouched.
…sonExceptionFileHandler recovers gracefully when the file contains invalid JSON, replacing it with valid content for the new record
…ileHandler behavior: the JSON output groups entries by module name. The test now checks the test_utilities_logs bucket, confirms the ValueError entry was recorded, and asserts the taskName matches the async task name.
…th a docstring in logs.py and replaced the hardcoded sleep values in __aexit__.
…Log.ASYNC_EXIT_SLEEP_SECONDS = 0.01 (restored afterward) so the test completes quickly.
ppinchuk
added
enhancement
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the logging test suite by adding comprehensive test coverage for COMPASS Ordinance logging utilities and introduces a configurable sleep constant to improve test performance.
- Adds
ASYNC_EXIT_SLEEP_SECONDSconstant toLocationFileLogclass for configurable async exit sleep timing - Introduces extensive test coverage for logging filters, formatters, handlers, and context managers
- Implements a test fixture to speed up async tests by reducing sleep times
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| compass/utilities/logs.py | Adds configurable ASYNC_EXIT_SLEEP_SECONDS constant to enable faster testing of async exit behavior |
| tests/python/unit/utilities/test_utilities_logs.py | Adds comprehensive test coverage for logging components including filters, formatters, handlers, and context managers; introduces fixture to accelerate async tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
castelao
approved these changes
Nov 15, 2025
Member
castelao
left a comment
castelao
left a comment
There was a problem hiding this comment.
Thanks for adding all these tests. The operations system I'm developing relies on the logs, so it is great to know that we can count on a robust logging.
rajeee
pushed a commit
that referenced
this pull request
May 27, 2026
* Add extra tests * Enhance async log listener test to capture emitted log records * Add check to exception filter for non-exception records * Refactor log record creation in tests to use a factory function * Add helper fixture to attach ValueError exc_info to log records * Refactor log record creation and exception handling in tests * Rename log record helper functions for clarity and consistency * Added test_json_exception_file_handler_existing_file in test_utilities_logs.py to ensure _JsonExceptionFileHandler leaves pre-existing JSON content untouched while appending new exception entries. * Added test_json_exception_file_handler_emit_type_error in test_utilities_logs.py to verify _JsonExceptionFileHandler.emit short-circuits cleanly when json.dumps raises TypeError, leaving the existing JSON file untouched. * Added test_json_exception_file_handler_invalid_json to verify that _JsonExceptionFileHandler recovers gracefully when the file contains invalid JSON, replacing it with valid content for the new record * Updated test_location_file_log_async_context to match _JsonExceptionFileHandler behavior: the JSON output groups entries by module name. The test now checks the test_utilities_logs bucket, confirms the ValueError entry was recorded, and asserts the taskName matches the async task name. * Introduced ASYNC_EXIT_SLEEP_SECONDS on LocationFileLog (default 3) with a docstring in logs.py and replaced the hardcoded sleep values in __aexit__. * In test_location_file_log_async_context, temporarily set LocationFileLog.ASYNC_EXIT_SLEEP_SECONDS = 0.01 (restored afterward) so the test completes quickly. * Add tests for empty handler * Minor updates * Use module-level fixture
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Overview
Improve test coverage for the JSON exception file handler and refactor log record creation in tests. Introduce a configurable sleep duration for async log flushing to enhance performance and maintainability.